Skip to content

fix(core,sdk): stop chat.agent losing messages during recovery - #4907

Open
ericallam wants to merge 12 commits into
mainfrom
feature/tri-13752-chatagent-version-handover-duplicates-messages-and-turns
Open

fix(core,sdk): stop chat.agent losing messages during recovery#4907
ericallam wants to merge 12 commits into
mainfrom
feature/tri-13752-chatagent-version-handover-duplicates-messages-and-turns

Conversation

@ericallam

Copy link
Copy Markdown
Member

Summary

When a chat.agent run boots to continue a session (a version handover, or a retry after a crash), it replays the unacknowledged user messages off session.in and dispatches them itself. A previous change stopped the live tail from re-answering those same messages by folding them into the resume cursor in one step. That cursor is what the next boot reads to know where to resume, and folding in every recovered message at once let it advance past a message the run had not answered yet. So if the run answered the first recovered message, wrote its turn boundary, then crashed before dispatching the rest, the next boot resumed past those messages and they were never answered.

Fix

A recovered message is now claimed on the session-stream router instead of folded into the cursor. A claim does two independent things:

  • it drops the message however late the live tail re-delivers it, so a recovered message is never answered twice;
  • it holds the resume cursor behind that message until the boot has dispatched it, so a turn boundary never publishes a cursor past a message still waiting for a turn.

The boot settles each claim as it dispatches the message, or right away for a message it folds into the seed chain or deliberately skips, so the cursor only advances over messages that have actually been handled. A claimed record whose route re-read never arrives over the tail degrades to being answered twice on the next boot, never to being dropped.

Covered by router-level unit tests for the claim/settle floor and a chat.agent boot test asserting the cursor published after the first recovered turn stays behind the still-unanswered ones.

…ashboard reader

Adds TranscriptSnapshotV2 (entries of {id, final, message} plus an opaque state record) and parseTranscriptSnapshot(), which reads a version 1 or version 2 blob into the version 2 shape. The Sessions dashboard seeds its transcript through that reader, so it keeps rendering history once the SDK starts writing version 2. The SDK still writes version 1 in this change.
…ptStorage seam

Introduces the TranscriptStorage interface (load/save over id-addressed changes: put, remove, truncateAfter, state) and makes the built-in snapshot writer its default implementation. The runtime keeps a shadow of the transcript it last saved and hands the storage the diff after every turn, failed turn and history-changing action, together with the stream cursors the next boot resumes from. The default storage reduces each changeset onto an in-memory copy and rewrites the blob as version 2, so a turn still costs one PUT and no GET, and the boot read goes through load.

The snapshot read/write helpers move to their own module so the storage can import them without a cycle; the test seams keep their import path. The mock chat agent harness now reports version 2 snapshots and accepts either version as a seed.
@changeset-bot

changeset-bot Bot commented Sep 8, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 2ece24d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 27 packages
Name Type
@trigger.dev/sdk Patch
@trigger.dev/core Patch
@trigger.dev/python Patch
@internal/dashboard-agent Patch
@trigger.dev/build Patch
trigger.dev Patch
@trigger.dev/redis-worker Patch
@trigger.dev/schema-to-json Patch
@internal/clickhouse Patch
@internal/llm-model-catalog Patch
@internal/metrics-pipeline Patch
@trigger.dev/rbac Patch
@internal/redis Patch
@internal/replication Patch
@internal/run-engine Patch
@internal/run-store Patch
@internal/schedule-engine Patch
@internal/tracing Patch
@internal/webhook-engine Patch
@internal/webhook-sources Patch
@internal/cache Patch
@trigger.dev/react-hooks Patch
@trigger.dev/rsc Patch
@trigger.dev/database Patch
@trigger.dev/otlp-importer Patch
@trigger.dev/sso Patch
@internal/testcontainers Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 43520ea3-e703-4948-847b-d345c6561001

📥 Commits

Reviewing files that changed from the base of the PR and between 52addef and 1268dba.

📒 Files selected for processing (1)
  • packages/trigger-sdk/src/v3/ai.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (41)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
  • GitHub Check: sdk-compat / Node.js 26.4 (warp-ubuntu-latest-x64-4x)
  • GitHub Check: sdk-compat / Node.js 24.18 (warp-ubuntu-latest-x64-4x)
  • GitHub Check: sdk-compat / Deno Runtime
  • GitHub Check: sdk-compat / Bun Runtime
  • GitHub Check: sdk-compat / Cloudflare Workers
  • GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
  • GitHub Check: internal / 🧪 Unit Tests: Internal
  • GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - pnpm)
  • GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
  • GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - npm)
  • GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
  • GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
  • GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
  • GitHub Check: runops-guard / runops-guard
  • GitHub Check: typecheck / typecheck
  • GitHub Check: code-quality / code-quality
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Build and publish previews
🧰 Additional context used
📓 Path-based instructions (7)
Always import from `@trigger.dev/sdk`.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • packages/trigger-sdk/src/v3/ai.ts
**Prefer static imports over dynamic imports.**

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • packages/trigger-sdk/src/v3/ai.ts
Add crumbs as you write code — not just when debugging.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • packages/trigger-sdk/src/v3/ai.ts
In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Files:

  • packages/trigger-sdk/src/v3/ai.ts
Use function declarations instead of default exports

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Files:

  • packages/trigger-sdk/src/v3/ai.ts
Use types over interfaces for TypeScript Avoid using enums; prefer string unions or const objects instead

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Files:

  • packages/trigger-sdk/src/v3/ai.ts
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs Do not use high-cardinality attributes in OTEL metr...

📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)

Files:

  • packages/trigger-sdk/src/v3/ai.ts
🔇 Additional comments (12)
packages/trigger-sdk/src/v3/ai.ts (12)

2322-2326: LGTM!

Also applies to: 2357-2363


7172-7173: LGTM!


7448-7452: LGTM!


7541-7542: LGTM!


7551-7564: Traced and correct. The seqNum lookup by message id correctly maps hookRecoveredTurns back to the original replayedInTail records (or leaves seqNum undefined for a hook-synthesized message that never had a record), and the default branches mirror the pre-existing splice/re-dispatch behavior exactly. No claim is left unaccounted for: every excluded entry is picked up by the settle loop at Line 7617-7621.


7595-7601: LGTM!


7610-7620: LGTM!


7804-7804: LGTM!


8055-8055: LGTM!


9636-9636: LGTM!


10012-10012: LGTM!


7270-7282: 🗄️ Data Integrity & Integration

SessionChannelRouter.settleRecovered(seqNum: number) removes only seqNum from #owed. It leaves #claimed unchanged, so late tail delivery remains suppressed. Since #owed is a Set, repeated calls are idempotent. Other owed sequences continue to hold the resume floor.


Walkthrough

The session router now tracks recovered sequence claims and unsettled claims separately. It drops late live-tail records for claimed sequences and holds the resume floor until each claim settles. Chat recovery passes individual sequence numbers to the router and settles dispatched or undispatched recovered records. Tests cover router floor behavior and prevent duplicate recovered chat messages and turns.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: 🔵 Low · up to 1268d

Recovery now claims and settles individual messages to prevent message loss, but a gap in recovered sequences can still advance a replay cutoff too far in a narrowed recovery path, potentially suppressing a message.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description clearly explains the problem, fix, behavior, and test coverage. However, it omits the required issue reference, checklist, Testing section, Changelog section, and Screenshots section f… Add a "Closes #" reference, complete the checklist, and add the required Testing, Changelog, and Screenshots sections. State "N/A" for Screenshots if no screenshots apply, and include the commands or steps used to verify the change u…
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: preventing chat.agent from losing messages during recovery.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (1 skipped: 1 too large.)

Full details: Description check

Explanation

The description clearly explains the problem, fix, behavior, and test coverage. However, it omits the required issue reference, checklist, Testing section, Changelog section, and Screenshots section from the repository template.

Resolution

Add a "Closes #<issue>" reference, complete the checklist, and add the required Testing, Changelog, and Screenshots sections. State "N/A" for Screenshots if no screenshots apply, and include the commands or steps used to verify the change under Testing.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/tri-13752-chatagent-version-handover-duplicates-messages-and-turns

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Sep 8, 2026

Copy link
Copy Markdown

Open in StackBlitz

@trigger.dev/build

npm i https://pkg.pr.new/@trigger.dev/build@2ece24d

trigger.dev

npm i https://pkg.pr.new/trigger.dev@2ece24d

@trigger.dev/core

npm i https://pkg.pr.new/@trigger.dev/core@2ece24d

@trigger.dev/python

npm i https://pkg.pr.new/@trigger.dev/python@2ece24d

@trigger.dev/react-hooks

npm i https://pkg.pr.new/@trigger.dev/react-hooks@2ece24d

@trigger.dev/redis-worker

npm i https://pkg.pr.new/@trigger.dev/redis-worker@2ece24d

@trigger.dev/rsc

npm i https://pkg.pr.new/@trigger.dev/rsc@2ece24d

@trigger.dev/schema-to-json

npm i https://pkg.pr.new/@trigger.dev/schema-to-json@2ece24d

@trigger.dev/sdk

npm i https://pkg.pr.new/@trigger.dev/sdk@2ece24d

commit: 2ece24d

coderabbitai[bot]

This comment was marked as resolved.

@ericallam
ericallam marked this pull request as ready for review September 8, 2026 11:57
devin-ai-integration[bot]

This comment was marked as resolved.

@ericallam
ericallam force-pushed the feature/tri-13752-chatagent-version-handover-duplicates-messages-and-turns branch from 7d29239 to 9a2cfbb Compare September 8, 2026 12:38
@ericallam
ericallam changed the base branch from main to feat/transcript-storage-tri-13667 September 8, 2026 12:38
@ericallam
ericallam force-pushed the feature/tri-13752-chatagent-version-handover-duplicates-messages-and-turns branch 3 times, most recently from 52addef to 1268dba Compare September 8, 2026 17:43
ericallam and others added 8 commits September 8, 2026 18:49
…nic session.in floor

A snapshot with no savedAt no longer drops the whole transcript: savedAt is
optional and defaults to 0 (it only orders snapshot history before live
chunks, so losing it should not lose the conversation).

The offline chat harness now models session.in as a durable stream whose
seqNums stay monotonic across a chat's runs, and the test session-stream
manager's setLastSeqNum only ever advances (matching the production manager).
Without this a fresh per-run manager restarted seqNums at 0, so a
continuation's follow-up message collided with the resume floor and was
dropped, hanging the turn loop.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VG39FXXkFFU24U5EtJMwPi
…ript storage state

The model lane after a compaction cannot be rebuilt from the transcript, so every continuation used to re-read the whole conversation and summarise it again. The runtime now records the compacted lane in the storage's state slot, with the transcript id it covers and a fingerprint of that prefix, and rebuilds from it at boot when the prefix is unchanged. A rollback or edit that reconverts the lane clears the state in the same changeset as the truncate.

Conversational messages added with chat.inject are recorded the same way, anchored to the transcript message they followed, so they survive a continuation instead of living only in the worker that received them.

Adds an in-memory storage that logs the changesets it receives, and a test-only override for the storage the runtime persists through, so the exact changesets for a turn, a mid-turn steer, a compaction, a rollback and an injection are asserted.
…t storage own the model's context

One condition used to decide three things at boot: whether to read the persisted transcript, whether to replay the session's output tail, and whether to replay unacknowledged input. Registering hydrateMessages switched all three off, so an app that owned its own context also lost crash recovery, and no application can rebuild the tail its dead run had already emitted. The replays and onRecoveryBoot now run for every agent; only the transcript read is skipped for hydrateMessages.

The storage can now declare loadContext, which the runtime calls on every turn and action in place of the accumulated transcript, the role hydrateMessages played, while save keeps receiving every change. hydrateMessages is deprecated with a one-time warning, and configuring it together with a storage that has loadContext is an error.
…nformance suite

chat.agent takes a storage option, with the platform snapshot as the default and the TranscriptStorage types, defaultStorage, snapshotTranscriptStorage, memoryTranscriptStorage and reduceTranscriptChanges exported from @trigger.dev/sdk/ai. chat.createLoadTranscriptAction(storage) reads a conversation on the server for any storage, and useLoadTranscript renders it in the browser and seeds the transport's resume cursor. runTranscriptStorageTests from @trigger.dev/sdk/ai/test is the contract a storage implementation has to meet.

A new secret-key endpoint, GET /api/v1/sessions/:id/transcript, pages the platform snapshot server-side so rendering the last page of a long conversation does not download all of it; the default storage uses it for paged reads and falls back to the whole blob.
… message rows

The real-schema conformance target for the storage contract: one row per message in chat_messages, with the runtime's state and cursors in two new nullable columns on chats. Paging is by position in SQL. The agent itself still persists through its hooks.
Treat a non-positive page limit as no limit instead of an empty page a
caller cannot tell apart from the end of the transcript. Hold a seeded
resume cursor until the session exists, so a transcript load that resolves
before the session is created still opens the live stream past the
persisted history instead of replaying it.
Consume a pending resume cursor in setSession() and stop seedResumeCursor
from moving an existing session's cursor backward, so a transcript load that
resolves around session creation, or after the live stream already advanced,
does not replay persisted events. Add conformance coverage for a partial
answer's non-final status, and drive the resume-cursor unit tests through a
real transport. Docs: loadContext also fires on actions, a custom storage is
not limited to row-per-message backends, and the bring-your-own example
imports anthropic.
@ericallam
ericallam force-pushed the feat/transcript-storage-tri-13667 branch from 2418fd0 to af81d57 Compare September 8, 2026 17:53
@ericallam
ericallam force-pushed the feature/tri-13752-chatagent-version-handover-duplicates-messages-and-turns branch from 1268dba to 95d3ab5 Compare September 8, 2026 17:55
saveTranscript made compaction and injections mutually exclusive in the
persisted runtime state, but restoreModelLane treats them independently:
injections anchored past a valid compaction apply on top, and if the
compaction fingerprint fails to match at boot the whole compacted lane is
discarded. In that case a same-turn injection that only lived in the
dropped `injections` field was lost. Emit both fields; restore already
filters injections that a valid compaction covers, so there is no
double-apply.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VG39FXXkFFU24U5EtJMwPi
@ericallam
ericallam force-pushed the feat/transcript-storage-tri-13667 branch from af81d57 to 5f06b37 Compare September 8, 2026 18:07
Rebased onto the transcript-storage stack. A continuation boot claims recovered session.in seqNums on the router and holds the resume cursor behind each until the boot settles it, so suppressing the tail's re-answer no longer advances the cursor past an un-answered message. Adds a changeset and router/boot tests.
@ericallam
ericallam force-pushed the feature/tri-13752-chatagent-version-handover-duplicates-messages-and-turns branch from 95d3ab5 to 2ece24d Compare September 8, 2026 18:09
@ericallam
ericallam added this pull request to the merge queue Sep 8, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue because a pull request earlier in the stack was removed Sep 8, 2026
@ericallam
ericallam force-pushed the feat/transcript-storage-tri-13667 branch from 5f06b37 to ed95a9e Compare September 9, 2026 06:20
Base automatically changed from feat/transcript-storage-tri-13667 to main September 9, 2026 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants